Skip to content

RSV bug fixes and other minor issues#13630

Merged
KarnaiahPesula merged 5 commits into
developmentfrom
13610-cryptosporidium-enhance-case-data-form
Nov 4, 2025
Merged

RSV bug fixes and other minor issues#13630
KarnaiahPesula merged 5 commits into
developmentfrom
13610-cryptosporidium-enhance-case-data-form

Conversation

@KarnaiahPesula
Copy link
Copy Markdown
Contributor

@KarnaiahPesula KarnaiahPesula commented Oct 24, 2025

Fixes #

Summary by CodeRabbit

  • New Features

    • Animal-location free-text and caption; Giardia exposure guidance; many new diseases (including RSV, Giardiasis, Cryptosporidiosis); exposure-type-aware classification criteria; auto-calculated ICU length of stay; event investigation date validations; symptoms grouping and numeric time-off-work field; pathogen test handling for RSV.
  • Bug Fixes

    • Corrected twin birth enum key; refined exposure description wording.
  • Chores

    • Removed immunodepression field and mappings; large database/schema additions and migrations.
  • Tests

    • Added classification tests for Giardiasis and Cryptosporidiosis.

@KarnaiahPesula KarnaiahPesula linked an issue Oct 24, 2025 that may be closed by this pull request
23 tasks
@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Oct 24, 2025

Walkthrough

Adds many disease constants (incl. RSV, GIARDIASIS, CRYPTOSPORIDIOSIS); changes classification to use explicit ExposureType checks; adds animalLocationText; removes immunodepression accessors; converts timeOffWorkDays String→Float; updates API, backend, UI, i18n, tests, and DB migration.

Changes

Cohort / File(s) Summary
Classification Exposure Criteria
sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java
Added constructor ClassificationExposureCriteriaDto(ExposureType); eval() supports exposure-type-only matches when propertyId == null; refactored reflection/value extraction; adjusted buildDescription() for exposure-type-only descriptions.
Case Classification Logic
sormas-backend/src/main/java/de/symeda/sormas/backend/caze/classification/CaseClassificationFacadeEjb.java
Reworked GIARDIASIS/CRYPTOSPORIDIOSIS criteria to use explicit ExposureType checks, added confirmed-symptom blocks, removed epiData-based compact checks, and added helper exposure(ExposureType).
Case Classification Tests
sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java
Added tests for Giardiasis and Cryptosporidiosis classification paths and extended test scaffolding.
Health Conditions — Immunodepression Removal
sormas-api/.../HealthConditionsDto.java, sormas-backend/.../HealthConditions.java, sormas-backend/.../HealthConditionsMapper.java, sormas-ui/.../HealthConditionsForm.java
Removed immunodepression constant/field/getter/setter from DTO/UI; removed entity accessors and mapper assignments.
Exposure — Animal Location Text
sormas-api/.../ExposureDto.java, sormas-backend/.../Exposure.java, sormas-backend/.../epidata/EpiDataFacadeEjb.java, sormas-ui/.../exposure/ExposureForm.java
Added animalLocationText constant/field with annotations & size constraint, getters/setters, mapped in EpiData facade, and integrated the new field and visibility into the UI form.
Person — Birth Place Diseases
sormas-api/src/main/java/de/symeda/sormas/api/person/PersonDto.java
Expanded @Diseases on placeOfBirth* fields to include RESPIRATORY_SYNCYTIAL_VIRUS alongside CONGENITAL_RUBELLA.
Symptoms — RSV and timeOffWorkDays type
sormas-api/.../SymptomsDto.java, sormas-backend/.../Symptoms.java, sormas-ui/.../SymptomsForm.java
Added @SymptomGrouping(RESPIRATORY) to wheezing; included RSV for asymptomatic; changed timeOffWorkDays from StringFloat in DTO/entity and added UI converter + error message.
Pathogen Test UI — RSV support
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java
Reordered TESTED_DISEASE_VARIANT layout row; added RSV to result-decision map allowing sequencing test types.
EpiData / Event / Hospitalization UI
sormas-ui/.../EpiDataForm.java, sormas-ui/.../EventDataForm.java, sormas-ui/.../HospitalizationForm.java
EpiDataForm: appended Giardia-specific info text for GIARDIASIS; EventDataForm: added date comparison validators for investigation dates; HospitalizationForm: compute/set ICU length of stay when both ICU dates present.
I18n / Captions / Strings
sormas-api/.../i18n/Captions.java, sormas-api/.../i18n/Strings.java, sormas-api/src/main/resources/captions.properties, sormas-api/src/main/resources/strings.properties
Added Exposure.animalLocationText caption/key; added classificationCriteriaRestrictedToExposureType and giardiaInfoExposureInvestigation strings; adjusted Exposure.animalLocation caption casing.
Enum / DB Schema / SQL Migration
sormas-api/src/main/resources/enum.properties, sormas-backend/src/main/resources/sql/sormas_schema.sql
Renamed MultipleBirth.TWINTWINS; DB migration adds animalLocationText, casts timeOffWorkDays to float, removes immunodepression mappings/access, adds many new tables/rights/history triggers and schema versioning entries.
Sormas-to-Sormas Validation
sormas-backend/src/main/java/de/symeda/sormas/backend/sormastosormas/data/validation/SormasToSormasDtoValidator.java
Added infraValidator-based country validation for epiData in validateEpiData; import reordering (non-functional).

Sequence Diagram(s)

sequenceDiagram
    participant UI as UI/Form
    participant Facade as CaseClassificationFacade
    participant Criteria as ClassificationExposureCriteriaDto
    participant Exposure as Exposure/EpiData
    participant DB as Backend/DB

    Note over Facade,Criteria: Exposure-type-aware classification flow

    UI->>Facade: submit case data
    Facade->>Criteria: exposure(ExposureType)
    Criteria->>Exposure: eval(entity, propertyId?)
    alt propertyId == null and exposureType matches
        Exposure-->>Criteria: match (exposure-type-only)
        Criteria-->>Facade: true
        Facade->>DB: apply PROBABLE rules
    else propertyId != null and property matches
        Exposure-->>Criteria: match (property + exposure)
        Criteria-->>Facade: true
        Facade->>Facade: evaluate confirmed symptoms/tests
        alt confirmed symptoms/tests
            Facade->>DB: set CONFIRMED
        else
            Facade->>DB: remain PROBABLE
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~50 minutes

  • Pay special attention to:
    • ClassificationExposureCriteriaDto (reflection paths and exposure-only matching).
    • CaseClassificationFacadeEjb (new GIARDIASIS/CRYPTOSPORIDIOSIS rules).
    • DB migration SQL (data type casts, new tables/rights, triggers, schema versioning).
    • Propagation of timeOffWorkDays type change across DTOs, entities, UI, and DB.

Possibly related PRs

  • #13624 — overlaps Giardiasis/Cryptosporidiosis classification and exposure-type changes.
  • #13564 — related symptoms/timeOffWorkDays and RSV handling changes.
  • #13581 — related UI changes for hospitalization/ICU date handling.

Suggested reviewers

  • obinna-h-n
  • raulbob

Poem

🐰 A hop, a nibble, fields anew,

Exposure types and RSV too,
Floats for days and tests aligned,
Animal notes and rules refined,
I thump my foot — the schema grew.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Description check ⚠️ Warning The pull request description only contains the template placeholder 'Fixes #' with no actual issue number, linked issues, or description of the changes made. Complete the description by specifying the issue number (e.g., 'Fixes #13610'), adding a summary of changes, and explaining the rationale for modifications like RSV field additions and classification logic updates.
Title check ❓ Inconclusive The title 'RSV bug fixes and other minor issues' is vague and generic, using non-specific terms like 'other minor issues' that obscure the actual changeset scope. Replace vague language with specific details about the main changes. For example: 'Add RSV support to place of birth fields and fix Giardiasis/Cryptosporidiosis classification' would be clearer.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 13610-cryptosporidium-enhance-case-data-form

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java (1)

2008-2008: Remove orphaned immunodepression caption from both source and generated files

The constant HealthConditions_immunodepression at line 2008 in Captions.java is indeed orphaned. Verification confirms:

  • The IMMUNODEPRESSION field is no longer present in HealthConditionsForm.java
  • No active Java code references the HealthConditions_immunodepression constant anywhere
  • The entry HealthConditions.immunodepression=Immunodepression exists in captions.properties
  • Since Captions.java is @Generated, the orphaned constant was generated from the properties file

Required actions:

  1. Remove HealthConditions.immunodepression=Immunodepression from sormas-api/src/main/resources/captions.properties
  2. Regenerate Captions.java using the I18nConstantGenerator to remove the orphaned constant
🧹 Nitpick comments (8)
sormas-backend/src/main/java/de/symeda/sormas/backend/symptoms/Symptoms.java (1)

2095-2101: Persist as float consistently and add basic range guard.

Align with other float fields (e.g., temperature) and prevent negative day counts.

Apply:

-	public Float getTimeOffWorkDays() {
+	@Column(columnDefinition = "float4")
+	public Float getTimeOffWorkDays() {
 		return timeOffWorkDays;
 	}
 
 	public void setTimeOffWorkDays(Float timeOffWorkDays) {
-		this.timeOffWorkDays = timeOffWorkDays;
+		// Disallow negatives; null is allowed
+		this.timeOffWorkDays = timeOffWorkDays != null && timeOffWorkDays < 0 ? 0f : timeOffWorkDays;
 	}

If JSR‑303 is in use on entities, prefer @PositiveOrZero on the getter instead of manual clamping.

sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.java (1)

193-195: Defensive normalize animalLocationText when not OTHER.

UI clears this when hidden, but API callers could submit inconsistent data. Consider nulling the text unless AnimalLocation.OTHER to keep invariants tight.

Within this method:

 		target.setAnimalLocation(source.getAnimalLocation());
-		target.setAnimalLocationText(source.getAnimalLocationText());
+		target.setAnimalLocationText(source.getAnimalLocationText());
+		// Normalize: keep free text only for OTHER
+		if (source.getAnimalLocation() != de.symeda.sormas.api.exposure.AnimalLocation.OTHER) {
+			target.setAnimalLocationText(null);
+		}
sormas-api/src/main/resources/captions.properties (1)

1580-1582: Trim leading spaces; i18n wiring confirmed.

The i18n integration is correctly implemented: Captions.java contains the constant "Exposure_animalLocationText", ExposureDto defines ANIMAL_LOCATION_TEXT with getters and setters, and the backend Exposure entity has the corresponding animalLocationText field. The leading spaces after '=' on lines 1581-1582 are inconsistent with the file's convention and should be removed.

-Exposure.animalLocation= Animal location
-Exposure.animalLocationText= Specify animal location
+Exposure.animalLocation=Animal location
+Exposure.animalLocationText=Specify animal location
sormas-backend/src/main/java/de/symeda/sormas/backend/exposure/Exposure.java (1)

142-142: Consider adding @column annotation for consistency.

The new animalLocationText field and accessors are implemented correctly. However, for consistency with other text fields like description (line 198) and exposureTypeDetails (line 217), consider adding a @Column(columnDefinition = "text") annotation to the getter method. This makes the intent explicit and ensures the column type is clearly defined as TEXT rather than VARCHAR.

Apply this diff to add the annotation:

+	@Column(columnDefinition = "text")
 	public String getAnimalLocationText() {
 		return animalLocationText;
 	}

Also applies to: 705-711

sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (3)

1242-1257: Method name typo: use “ruleOut” (not “rollOut”).
Keeps test names consistent with the suite.

Apply:

- public void rollOutFalsePositivesForGiardiasis() {
+ public void ruleOutFalsePositivesForGiardiasis() {

1271-1276: Limit created test types to those relevant to confirmation.
Keeps fixtures lean and avoids noise from unrelated assays.

Consider:

- createSampleTestsForAllTestTypesExcept(
-   caze,
-   Disease.GIARDIASIS,
-   PathogenTestType.NEUTRALIZING_ANTIBODIES,
-   PathogenTestType.ENZYME_LINKED_IMMUNOSORBENT_ASSAY,
-   PathogenTestType.ANTIBIOTIC_SUSCEPTIBILITY);
+ // Create only the confirmatory ones explicitly:
+ creator.createPathogenTest(caze, Disease.GIARDIASIS, PathogenTestType.MICROSCOPY, PathogenTestResultType.POSITIVE);
+ creator.createPathogenTest(caze, Disease.GIARDIASIS, PathogenTestType.PCR_RT_PCR, PathogenTestResultType.POSITIVE);

1282-1295: Method name typo: use “ruleOut” (not “rollOut”).
Mirror other diseases.

- public void rollOutFalsePositivesForCryptosporidiosis() {
+ public void ruleOutFalsePositivesForCryptosporidiosis() {
sormas-backend/src/main/java/de/symeda/sormas/backend/caze/classification/CaseClassificationFacadeEjb.java (1)

544-549: Giardiasis probable: exposure-type criteria look right.
Using ANIMAL_CONTACT/RECREATIONAL_WATER/FOOD/FLOOD_EXPOSURE/SEXUAL_CONTACT matches the intended epi signals. Ensure ClassificationExposureCriteriaDto(exposureType) checks presence-of-type irrespective of subfields.

Optionally add brief Javadoc on exposure(ExposureType) to document semantics.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f89986 and e7496af.

📒 Files selected for processing (25)
  • sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java (3 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/clinicalcourse/HealthConditionsDto.java (0 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.java (3 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java (1 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/i18n/Strings.java (2 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/person/PersonDto.java (1 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java (4 hunks)
  • sormas-api/src/main/resources/captions.properties (1 hunks)
  • sormas-api/src/main/resources/enum.properties (1 hunks)
  • sormas-api/src/main/resources/strings.properties (2 hunks)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/caze/classification/CaseClassificationFacadeEjb.java (2 hunks)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/clinicalcourse/HealthConditions.java (0 hunks)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/clinicalcourse/HealthConditionsMapper.java (0 hunks)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.java (2 hunks)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/exposure/Exposure.java (2 hunks)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/sormastosormas/data/validation/SormasToSormasDtoValidator.java (2 hunks)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/symptoms/Symptoms.java (2 hunks)
  • sormas-backend/src/main/resources/sql/sormas_schema.sql (1 hunks)
  • sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (14 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.java (2 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (3 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/events/EventDataForm.java (1 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposureForm.java (4 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/hospitalization/HospitalizationForm.java (1 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java (2 hunks)
💤 Files with no reviewable changes (3)
  • sormas-backend/src/main/java/de/symeda/sormas/backend/clinicalcourse/HealthConditionsMapper.java
  • sormas-backend/src/main/java/de/symeda/sormas/backend/clinicalcourse/HealthConditions.java
  • sormas-api/src/main/java/de/symeda/sormas/api/clinicalcourse/HealthConditionsDto.java
🧰 Additional context used
🧬 Code graph analysis (4)
sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/I18nProperties.java (1)
  • I18nProperties (39-536)
sormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposureForm.java (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/utils/FieldHelper.java (1)
  • FieldHelper (56-1270)
sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/I18nProperties.java (1)
  • I18nProperties (39-536)
sormas-ui/src/main/java/de/symeda/sormas/ui/events/EventDataForm.java (1)
sormas-ui/src/main/java/de/symeda/sormas/ui/utils/FieldHelper.java (1)
  • FieldHelper (56-1270)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Lint Code Base
  • GitHub Check: android app test (26)
  • GitHub Check: android app test (28)
  • GitHub Check: android app test (27)
  • GitHub Check: SORMAS CI
🔇 Additional comments (47)
sormas-ui/src/main/java/de/symeda/sormas/ui/samples/PathogenTestForm.java (2)

106-106: Layout change moves disease variant field after test result.

The repositioning of the disease variant field improves the logical flow of the form. This is a UI-only change with no functional impact.


1097-1098: No issues found after verification.

The Disease.RESPIRATORY_SYNCYTIAL_VIRUS enum constant is properly defined and the implementation correctly follows the established pattern used for other diseases in the same map. Both test types (SEQUENCING and WHOLE_GENOME_SEQUENCING) are valid for RSV per the codebase annotations.

sormas-backend/src/main/java/de/symeda/sormas/backend/sormastosormas/data/validation/SormasToSormasDtoValidator.java (1)

127-127: Code change is correct and follows established patterns.

The country validation for EpiData aligns with existing country validations throughout the class (e.g., Person.birthCountry, Person.citizenship, Location.country) and properly handles null values via the validateInfra method, which returns early for null references. No breaking changes for existing records.

sormas-ui/src/main/java/de/symeda/sormas/ui/epidata/EpiDataForm.java (2)

35-35: LGTM!

The import is necessary for StringUtils.EMPTY used on line 237 and is consistent with existing usage patterns in the codebase.


236-237: Confirm whether Cryptosporidiosis should receive the same treatment as Giardiasis in exposure investigation text.

The i18n key Strings.giardiaInfoExposureInvestigation exists and is properly defined. However, the code at lines 236–237 only provides disease-specific info text for GIARDIASIS; CRYPTOSPORIDIOSIS falls back to StringUtils.EMPTY. This differs from the codebase pattern where both diseases are typically handled together (e.g., SymptomsForm, HospitalizationForm, ExposureForm).

Verify whether this asymmetry is intentional or if CRYPTOSPORIDIOSIS should also receive similar info text (requiring a new i18n key and conditional logic).

sormas-ui/src/main/java/de/symeda/sormas/ui/events/EventDataForm.java (2)

254-255: LGTM! Captures field references for validation.

Capturing the return values from addField enables the validator wiring added below. This change is correct and necessary.


262-263: Original review comment contains fundamental misunderstandings about parameter semantics and null handling—code is largely correct.

The analysis in the original comment is incorrect regarding key concerns:

  1. Null handling (INCORRECT): The validator explicitly allows null values. The isValidValue() method returns true when either field is null, meaning the validation passes. The false parameter is not about null allowance.

  2. Parameter semantics (INCORRECT): The third false parameter controls date-only vs. datetime comparison via DateComparator, not null handling. It's correctly set to false to enable datetime comparisons.

  3. Visibility handling (INCORRECT): The investigation date fields use FieldHelper.setVisibleWhen() based on investigation status, not clearOnHidden=true. Fields remain in the form regardless of visibility.

The real concern worth noting: Line 263 compares startDate (DateTimeField) with investigationStartDate (DateField), which is a type mismatch. Verify this works correctly with Vaadin's field abstraction.

Likely an incorrect or invalid review comment.

sormas-ui/src/main/java/de/symeda/sormas/ui/clinicalcourse/HealthConditionsForm.java (1)

92-92: LGTM - Coordinated removal of IMMUNODEPRESSION field

The removal of IMMUNODEPRESSION from both the UI layout (line 92) and fields list (line 120) is clean and consistent with the cross-layer changes described in the PR summary. The layout remains reasonably balanced with the second column now containing 11 items.

Note: This is a breaking change that removes a health condition option from the UI. Users will no longer be able to view or edit this field, and any existing data in this field would have been handled by the database migration.

Also applies to: 120-120

sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java (1)

1810-1810: LGTM - New caption constant added correctly

The new Exposure_animalLocationText constant is properly placed in alphabetical order and follows the established naming convention. This aligns with the animalLocationText field addition across the API, backend, and UI layers.

sormas-api/src/main/java/de/symeda/sormas/api/symptoms/SymptomsDto.java (3)

2221-2222: LGTM! Proper symptom grouping for wheezing.

The addition of the RESPIRATORY symptom grouping annotation is appropriate and aligns with the respiratory nature of wheezing, especially in the context of RSV support.


2613-2614: LGTM! RSV can present asymptomatically.

The addition of RESPIRATORY_SYNCYTIAL_VIRUS to the asymptomatic field is clinically appropriate, as RSV infections can indeed be asymptomatic, particularly in mild cases.


2680-2680: Migration from String to Float for timeOffWorkDays is properly implemented.

The database schema includes a comprehensive migration that:

  • Validates existing String values against numeric format ^-?[0-9]*\.?[0-9]+$
  • Converts invalid/empty strings to NULL
  • Performs explicit type conversion with CAST(timeoffworkdays AS float4) for both symptoms and symptoms_history tables

All API consumers are aligned: the DTO, backend entity, and facade layer all use Float type with explicit mappings in SymptomsFacadeEjb.fillOrBuildEntity() and toSymptomsDto(). The UI form correctly uses TextField for numeric input. Edge cases (empty strings, non-numeric values, null preservation) are handled by the schema validation logic.

sormas-api/src/main/java/de/symeda/sormas/api/person/PersonDto.java (1)

221-222: LGTM! Consistent RSV support for birth location fields.

The addition of RESPIRATORY_SYNCYTIAL_VIRUS to all six birth-related fields is appropriate and consistent. RSV commonly affects newborns and infants, making birth location data relevant for epidemiological tracking, similar to the existing CONGENITAL_RUBELLA usage.

Also applies to: 226-227, 231-232, 237-238, 242-243, 248-249

sormas-backend/src/main/java/de/symeda/sormas/backend/epidata/EpiDataFacadeEjb.java (1)

356-358: LGTM: DTO mapping includes animalLocationText.

Round‑trip coverage looks correct.

sormas-ui/src/main/java/de/symeda/sormas/ui/exposure/ExposureForm.java (1)

48-49: animalLocationText is fully wired across all layers and ready to merge.

Verification confirmed:

  • ExposureDto: constant (line 116) + field + accessors (lines 278, 862-866)
  • Exposure backend: field + accessors (lines 142, 706-710)
  • Captions: constant (line 1810) + properties entry (line 1581)
  • UI bindings: correctly gated to AnimalLocation.OTHER (line 359)
sormas-api/src/main/resources/strings.properties (2)

102-102: LGTM!

The new i18n key provides clear labeling for exposure-type-only classification criteria.


1114-1114: LGTM!

The Giardia-specific exposure investigation guidance follows the established pattern for informational messages.

sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java (2)

44-47: LGTM!

The new constructor properly supports exposure-type-only classification criteria for diseases like Giardiasis and Cryptosporidiosis.


106-113: LGTM!

The conditional logic correctly handles both cases: criteria with property-specific checks and exposure-type-only criteria. The null check on line 105 prevents NPE when calling exposureType.toString().

sormas-backend/src/main/resources/sql/sormas_schema.sql (7)

14724-14725: Verify necessity and safety of constraint drop/re-add pattern.

The constraint is dropped and then immediately re-added with the same definition. This approach carries risks:

  • It can fail if foreign key relationships reference the constraint.
  • It's unusual to drop and recreate an identical constraint unless the constraint definition is being changed.

Confirm that this is necessary (e.g., to fix a constraint issue) and safe given existing data dependencies. If the goal is to ensure the constraint exists idempotently, consider whether a single ADD CONSTRAINT IF NOT EXISTS statement would be more appropriate (available in recent PostgreSQL versions).

Also applies to: 14731-14732


14726-14726: Validate the regex pattern for timeoffworkdays data migration.

The regex pattern '^\s*-?[0-9]*\.?[0-9]+\s*$' is used to identify valid numeric values before casting to float4. Confirm it correctly handles:

  • Valid formats: 123, 123.45, -456.78, .5, -.5
  • Invalid formats: empty strings, letters, exponential notation (1e-5), NaN, Infinity
  • Edge cases: whitespace-only strings, multiple dots, trailing/leading spaces

If the pattern is insufficient, some invalid data could slip through and cause the USING timeoffworkdays::float4 cast to fail.

Also applies to: 14733-14733


14726-14727: Confirm timeoffworkdays data migration is safe and intentional.

Changing timeoffworkdays from String to float4 is a significant schema change. Verify:

  • All existing data has been validated or migrated to numeric format.
  • The UPDATE statement (line 14726/14733) correctly nulls out all unparseable values without data loss.
  • Any application code consuming this field has been updated to handle float instead of String.
  • No business logic depends on string-specific behavior (e.g., leading zeros, custom formatting).

Also applies to: 14733-14734


14728-14728: Confirm immunodepression column removal is complete and safe.

Dropping immunodepression from healthconditions (and its history table) is permanent. Verify:

  • All data in this column has been migrated or archived if needed.
  • No active application code or queries reference this column.
  • The removal is intentional and aligns with the backend changes mentioned in the AI summary (removal from HealthConditions).

Also applies to: 14735-14735


14723-14723: LGTM: animallocationtext column addition.

Adding the animallocationtext varchar(255) column to both exposures and exposures_history tables is straightforward and properly mirrored. This aligns with the backend enhancements mentioned in the AI summary.

Also applies to: 14730-14730


14730-14735: LGTM: History table mirroring.

All changes to the current tables (exposures, epidata, symptoms, healthconditions) are properly replicated in their corresponding history tables, which is essential for audit trail integrity and consistency.


14737-14737: LGTM: Schema versioning.

Schema version 594 is properly inserted with descriptive comments and issue references (#13540, #13613), maintaining good change tracking practices.

sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (17)

506-510: Formatting-only change looks good.
No behavior change; keeps intent of excluding confirm-enabling tests.


829-833: Formatting-only change looks good.
Same semantics, clearer wrapping.


952-953: Correct exclusion set for monkeypox rule-out.
Excluding IGM/PCR/ISOLATION prevents inadvertent confirmation.


1057-1058: Plague rule-out exclusions are correct.
Avoids probable/confirmed via antigen detection/iso/PCR.


1106-1107: Stylistic split only; OK.
No functional impact.


1132-1137: IMI rule-out exclusions are appropriate.
Prevents confirmation via microscopy/PCR/antigen.


1146-1151: IMI probable rule-out block OK.
Prevents upgrade via confirmatory tests.


1232-1237: IPI rule-out: assertion updated to NOT_CLASSIFIED is correct.
IPI has only confirmed pathway; with confirmatory tests excluded, NOT_CLASSIFIED is expected.

Also applies to: 1239-1240


1260-1270: Giardiasis: minimal-probable path exercised correctly.
Asymptomatic→NO, GI symptom + animal contact → PROBABLE; good.


1278-1279: Confirmed assertion is valid after adding confirmatory tests.
Good.


1298-1301: Cryptosporidiosis: initial NOT_CLASSIFIED check is sensible.
Starts from asymptomatic.


1304-1317: Cryptosporidiosis: PROBABLE → CONFIRMED flow looks correct.
GI symptoms + exposure → PROBABLE, then confirmatory tests upgrade.


1355-1355: Formatting-only change looks good.
No behavior impact.


1436-1437: Formatting-only change looks good.
No behavior impact.


1567-1569: Adding GIARDIASIS/CRYPTOSPORIDIOSIS to suspect basis as no-ops is fine.
Keeps construction uniform.


1624-1627: No-op suspect builder branches for GIARDIASIS/CRYPTOSPORIDIOSIS are acceptable.
Default suspect criteria come from tests.


1673-1676: IMI confirmed basis enriched with microscopy and meningeal signs.
Good helper for confirmed scenarios.

sormas-backend/src/main/java/de/symeda/sormas/backend/caze/classification/CaseClassificationFacadeEjb.java (4)

731-733: Nice helper overload for exposure-by-type.
Improves readability of criteria definitions.


551-556: Giardiasis confirmed: symptom gate before confirmatory tests is appropriate.
Keeps confirmation clinically grounded.


564-569: Cryptosporidiosis probable: exposure set is comprehensive.
Inclusion of SYMPTOMATIC_CONTACT is sensible.


571-575: Cryptosporidiosis confirmed: aligns with microscopy/PCR/culture.
Looks consistent with tests.

@sormas-vitagroup
Copy link
Copy Markdown
Contributor

@SORMAS-Foundation SORMAS-Foundation deleted a comment from coderabbitai Bot Oct 28, 2025
Minor review comments.
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

♻️ Duplicate comments (1)
sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (1)

1649-1652: Critical: Missing break statement causes switch fall-through to exception.

The GIARDIASIS and CRYPTOSPORIDIOSIS cases are missing a break statement, causing execution to fall through to the default case which immediately throws UnsupportedOperationException. This will cause test failures for both diseases.

Apply this diff:

 	case GIARDIASIS:
 	case CRYPTOSPORIDIOSIS:
 		caze.getEpiData().getExposures().add(creator.buildAnimalContactExposure(TypeOfAnimal.CAT));
+		break;
 	default:
🧹 Nitpick comments (2)
sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java (1)

598-600: Converter setup is correct; consider adding range validation.

The StringToFloatConverter and custom error message are properly configured to handle the String-to-Float conversion for the TIME_OFF_WORK_DAYS field.

Consider adding validation to ensure the value is non-negative and within a reasonable range (e.g., 0-365 days):

timeOffWorkDaysField.setConverter(new StringToFloatConverter());
timeOffWorkDaysField
    .setConversionError(I18nProperties.getValidationError(Validations.onlyDecimalNumbersAllowed, timeOffWorkDaysField.getCaption()));
timeOffWorkDaysField.addValidator(value -> {
    if (value != null) {
        Float floatValue = (Float) value;
        if (floatValue < 0) {
            throw new InvalidValueException(I18nProperties.getValidationError(Validations.numberTooSmall, timeOffWorkDaysField.getCaption(), 0));
        }
        if (floatValue > 365) {
            throw new InvalidValueException(I18nProperties.getValidationError(Validations.numberTooBig, timeOffWorkDaysField.getCaption(), 365));
        }
    }
});

Note: You'll need to import com.vaadin.v7.data.validator.AbstractValidator or use an appropriate validator implementation, and verify the validation message keys exist in your i18n resources.

sormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.java (1)

275-279: Consider adding @sensitivedata annotation.

The field animalLocationText is missing the @sensitivedata annotation that is present on similar free-text fields in this class (e.g., animalContactTypeDetails at line 268, typeOfPlaceDetails at line 149, sexualExposureText at line 386, rawFoodContactText at line 395). Since this field can contain free-form location descriptions that may include identifying or sensitive information, it should be marked as sensitive data for proper handling and pseudonymization.

Apply this diff to add the annotation:

 	@Diseases({
 		Disease.GIARDIASIS,
 		Disease.CRYPTOSPORIDIOSIS })
+	@SensitiveData
 	@Size(max = FieldConstraints.CHARACTER_LIMIT_TEXT, message = Validations.textTooLong)
 	private String animalLocationText;
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e7496af and 922b704.

📒 Files selected for processing (6)
  • sormas-api/src/main/java/de/symeda/sormas/api/Disease.java (2 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java (3 hunks)
  • sormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.java (3 hunks)
  • sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (14 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/hospitalization/HospitalizationForm.java (1 hunks)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • sormas-ui/src/main/java/de/symeda/sormas/ui/hospitalization/HospitalizationForm.java
🧰 Additional context used
🧬 Code graph analysis (2)
sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/I18nProperties.java (1)
  • I18nProperties (39-536)
sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java (1)
sormas-api/src/main/java/de/symeda/sormas/api/i18n/I18nProperties.java (1)
  • I18nProperties (39-536)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: SORMAS CI
  • GitHub Check: android app test (27)
  • GitHub Check: android app test (26)
  • GitHub Check: android app test (28)
  • GitHub Check: Lint Code Base
🔇 Additional comments (13)
sormas-ui/src/main/java/de/symeda/sormas/ui/symptoms/SymptomsForm.java (1)

60-60: LGTM! Import is necessary for the converter usage below.

The import of StringToFloatConverter is appropriate for converting the TIME_OFF_WORK_DAYS TextField value to Float, aligning with the backend type change.

sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (4)

1567-1569: LGTM!

The addition of GIARDIASIS and CRYPTOSPORIDIOSIS cases with empty implementations is correct and consistent with other diseases in this method that have no special requirements for suspect case basis (e.g., CORONAVIRUS, INVASIVE_MENINGOCOCCAL_INFECTION).


1625-1627: LGTM!

The addition of GIARDIASIS and CRYPTOSPORIDIOSIS cases with empty implementations is correct and consistent with the pattern used for diseases with no additional suspect case requirements.


1259-1279: Review comment is incorrect—GIARDIASIS is properly handled.

The switch statement at lines 1649–1652 properly handles GIARDIASIS with both a case handler (that adds CAT animal contact exposure) and an explicit break statement. There is no fall-through bug. The buildProbableCaseBasis method will not throw UnsupportedOperationException for GIARDIASIS, and the test should execute without the claimed failure.

Likely an incorrect or invalid review comment.


1297-1318: Review comment is incorrect.

The test testAutomaticClassificationForCryptosporidiosis does not call buildProbableCaseBasis. It constructs the case manually using creator.createUnclassifiedCase() and directly sets symptoms and exposures. Therefore, any switch statement issue in buildProbableCaseBasis cannot affect this test's execution.

Likely an incorrect or invalid review comment.

sormas-api/src/main/java/de/symeda/sormas/api/Disease.java (3)

28-28: LGTM: AFP follow-up configuration updated.

The change enables follow-up tracking for AFP cases with a 60-day duration, which aligns with standard AFP surveillance practices for monitoring and investigation.


91-92: LGTM: New disease constants for GIARDIASIS and CRYPTOSPORIDIOSIS.

The configuration for both diseases is appropriate:

  • Follow-up enabled with 14-day duration (suitable for gastrointestinal diseases)
  • Case surveillance enabled
  • No variant tracking or extended classification (correct for these diseases)

73-75: Verify intentionality and data migration impact of the INFLUENZA hierarchy inversion.

The change reverses the default-active status between INFLUENZA (now active) and its subtypes INFLUENZA_A/B (now inactive). No test coverage was found validating this behavior, and the DiseaseConfigurationFacadeEjb fallback mechanism applies disease.isDefaultActive() to any existing DiseaseConfiguration records with null ACTIVE field—meaning existing data will silently adopt the new defaults. Confirm this is intentional and review potential impact on existing installations, particularly for case classification and disease filtering logic.

sormas-api/src/main/java/de/symeda/sormas/api/exposure/ExposureDto.java (2)

116-116: LGTM: Constant added following established conventions.

The ANIMAL_LOCATION_TEXT constant is properly positioned adjacent to the related ANIMAL_LOCATION constant and follows the naming pattern used throughout the class.


862-868: LGTM: Getter and setter properly implemented.

The accessor methods follow JavaBean conventions and are positioned logically adjacent to the related animalLocation methods.

sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java (3)

44-47: LGTM: New constructor supports exposure-type-only criteria.

The constructor correctly initializes the exposureType field, enabling classification based solely on exposure type without requiring a specific property. This aligns with the use case for Giardiasis and Cryptosporidiosis mentioned in the code comments.


64-68: LGTM: Early return optimizes exposure-type-only matching.

The early return correctly handles cases where only the exposure type matters (no specific property to check). The comment clearly documents the use case for Giardiasis and Cryptosporidiosis.


85-92: LGTM: Exception handling appropriately wraps reflection errors.

The try-catch block correctly handles reflection-related exceptions and converts them to RuntimeException, which is a standard pattern for reflection-based code that doesn't want to expose checked exceptions to callers.

Comment on lines +69 to 83
if (exposureType != null && exposure.getExposureType() != exposureType) {
continue;
}
Method method;
try {
method = getInvokeClass().getMethod("is" + propertyId.substring(0, 1).toUpperCase() + propertyId.substring(1));
} catch (NoSuchMethodException newE) {
throw new RuntimeException(newE);
method = getInvokeClass().getMethod("get" + propertyId.substring(0, 1).toUpperCase() + propertyId.substring(1));
} catch (NoSuchMethodException e) {
try {
method = getInvokeClass().getMethod("is" + propertyId.substring(0, 1).toUpperCase() + propertyId.substring(1));
} catch (NoSuchMethodException newE) {
throw new RuntimeException(newE);
}
} catch (SecurityException e) {
throw new RuntimeException(e);
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Potential NullPointerException when both propertyId and exposureType are null.

The code assumes propertyId is non-null when reaching line 74, but this isn't guaranteed. If an instance is created using the default constructor (line 40) or with both fields null, and the exposure has a non-null type, the code will throw a NullPointerException when attempting propertyId.substring(...).

Trace:

  1. Both propertyId and exposureType are null
  2. Line 66: Early return condition fails (if exposure has a type)
  3. Line 69: Filter condition is false (exposureType is null)
  4. Line 74: propertyId.substring(...) → NPE

Consider adding a validation check:

 			} else {
 				if (exposureType != null && exposure.getExposureType() != exposureType) {
 					continue;
 				}
+				if (propertyId == null) {
+					continue; // or throw IllegalStateException
+				}
 				Method method;
 				try {

Alternatively, validate in constructors that at least one of propertyId or exposureType is non-null.

🤖 Prompt for AI Agents
In
sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java
around lines 69 to 83, the code uses propertyId.substring(...) without guarding
against propertyId being null which can cause an NPE; fix by adding a null check
before attempting to build the getter name (e.g., if propertyId is null then
skip this reflection block or handle it consistently), or alternatively enforce
via constructors/validation that at least one of propertyId or exposureType is
non-null and throw an IllegalArgumentException early; ensure any added branch
either continues the loop when propertyId is null or throws a clear exception so
substring(...) is never called on a null reference.

Comment on lines +104 to +111
if (propertyId != null) {
sb.append(I18nProperties.getPrefixCaption(ExposureDto.I18N_PREFIX, propertyId));
sb.append(" ").append(I18nProperties.getString(Strings.classificationCriteriaForExposureType)).append(exposureType.toString());
} else {
sb.append(I18nProperties.getString(Strings.classificationCriteriaRestrictedToExposureType))
.append(" ")
.append(exposureType.toString());
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Verify i18n string keys exist in resource files.

The description logic correctly handles both cases (with and without propertyId). However, please confirm that the following i18n keys are defined in the resource bundles:

  • Strings.classificationCriteriaForExposureType (line 106)
  • Strings.classificationCriteriaRestrictedToExposureType (line 108)

🏁 Script executed:

#!/bin/bash
# Verify the i18n string keys exist in the codebase

echo "Searching for classificationCriteriaForExposureType..."
rg -n "classificationCriteriaForExposureType" --type java --type properties

echo -e "\nSearching for classificationCriteriaRestrictedToExposureType..."
rg -n "classificationCriteriaRestrictedToExposureType" --type java --type properties

Length of output: 189


🏁 Script executed:

#!/bin/bash
# Find properties files and search for the i18n keys

echo "Finding all .properties files..."
fd -e properties

echo -e "\n=== Searching for classificationCriteriaForExposureType ==="
rg -n "classificationCriteriaForExposureType"

echo -e "\n=== Searching for classificationCriteriaRestrictedToExposureType ==="
rg -n "classificationCriteriaRestrictedToExposureType"

echo -e "\n=== Checking Strings enum definitions ==="
rg -n "classificationCriteria" --type java -A 1 -B 1 | head -50

Length of output: 38589


🏁 Script executed:

#!/bin/bash
# Verify classificationCriteriaRestrictedToExposureType exists in all language property files

echo "=== Searching for classificationCriteriaRestrictedToExposureType in language files ==="
rg -c "classificationCriteriaRestrictedToExposureType" sormas-api/src/main/resources/strings*.properties

echo -e "\n=== Total count ==="
rg "classificationCriteriaRestrictedToExposureType" sormas-api/src/main/resources/ | wc -l

echo -e "\n=== Files WITH the key ==="
fd "strings.*properties" sormas-api/src/main/resources/ --exec rg -l "classificationCriteriaRestrictedToExposureType" {} \; | wc -l

Length of output: 190


Add translations for classificationCriteriaRestrictedToExposureType to all language property files.

The verification reveals a critical i18n gap: Strings.classificationCriteriaRestrictedToExposureType is defined only in the base strings.properties file and is missing from all 40+ language-specific translation files (strings_*.properties). While classificationCriteriaForExposureType has proper translations across all locales, the second key used at line 108 will fall back to untranslated text in non-English locales.

Add the key and its translations to each sormas-api/src/main/resources/strings_*.properties file to complete the i18n implementation.

🤖 Prompt for AI Agents
In
sormas-api/src/main/java/de/symeda/sormas/api/caze/classification/ClassificationExposureCriteriaDto.java
around lines 104-111, the i18n key
Strings.classificationCriteriaRestrictedToExposureType used when propertyId is
null exists only in strings.properties and is missing from all language-specific
files; add the key classificationCriteriaRestrictedToExposureType with
appropriate translated values to every
sormas-api/src/main/resources/strings_*.properties file (one entry per file),
using the same phrasing and placeholder semantics as
classificationCriteriaForExposureType translations to ensure consistent
localized output across all locales.

@sormas-vitagroup
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 922b704 and cb73faf.

📒 Files selected for processing (1)
  • sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: android app test (26)
  • GitHub Check: android app test (27)
  • GitHub Check: android app test (28)
  • GitHub Check: SORMAS CI
🔇 Additional comments (4)
sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (4)

1242-1279: LGTM: Giardiasis test methods are well-structured.

The test methods for Giardiasis follow established patterns and correctly verify classification logic. The use of the asymptomatic flag to prevent classification is appropriate, and the progression through NOT_CLASSIFIED → PROBABLE → CONFIRMED is logically sound.


1281-1318: LGTM: Cryptosporidiosis test methods are well-structured.

The test methods for Cryptosporidiosis correctly verify the classification logic and follow the established testing patterns. The use of different animal exposures (DOG here vs CAT in buildProbableCaseBasis) may be epidemiologically appropriate for testing different transmission vectors.


1567-1569: LGTM: Switch case additions for GIARDIASIS and CRYPTOSPORIDIOSIS are correct.

All switch cases include proper break statements and appropriate logic for each helper method. The empty case bodies in buildSuspectCaseBasis and buildSuspectCase are correct, as these diseases don't require special setup at those classification levels.

Also applies to: 1625-1627, 1649-1652


506-510: LGTM: Formatting improvements enhance readability.

The multi-line formatting of createSampleTestsForAllTestTypesExcept calls improves readability and is consistent with formatting patterns used elsewhere in the file.

Also applies to: 829-833, 952-953, 1057-1058, 1106-1107, 1132-1137, 1146-1151, 1232-1237, 1355-1355, 1436-1437

@sormas-vitagroup
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between cb73faf and d0c47e4.

📒 Files selected for processing (1)
  • sormas-backend/src/main/resources/sql/sormas_schema.sql (3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: android app test (26)
  • GitHub Check: android app test (27)
  • GitHub Check: android app test (28)
  • GitHub Check: SORMAS CI
  • GitHub Check: Lint Code Base
🔇 Additional comments (6)
sormas-backend/src/main/resources/sql/sormas_schema.sql (6)

14447-14457: LGTM!

Clean column additions with proper history table synchronization for Doctors declaration feature. All columns are nullable and use IF NOT EXISTS for idempotency.


14570-14596: LGTM!

Well-structured addition of Measles customization columns across multiple tables. Boolean defaults (vaccinationproposed, immuneglobulinproposed) are appropriate; other nullable columns follow expected patterns. History tables properly synchronized.


14651-14657: LGTM!

Three-step approach (ADD with DEFAULT → UPDATE NULLs → SET NOT NULL) correctly ensures data integrity before applying the NOT NULL constraint. History table handled consistently.


14668-14720: LGTM!

Systematic expansion of schema for Giardiasis/Cryptosporidiosis support. Type conversion for weightlossamount (integer → float4) correctly uses USING clause. Country_id foreign key column added, with constraint properly established in subsequent migration. All history tables synchronized.


14728-14729: Column removal verified; backend code is clean.

Verification confirms the immunodepression column removal is complete:

  • Entity field removed
  • No accessor methods (getters/setters)
  • Mapper contains no references
  • No UI form references

Backend code is properly updated and safe. The only orphaned artifacts are unused i18n caption entries (captions.properties and Captions.java), which are non-functional and can be cleaned up separately.


14724-14725: The review comment mischaracterizes the constraint being added.

The code adds only a FOREIGN KEY constraint to the nullable country_id column (line 14686 adds it as bigint without NOT NULL). A NOT NULL constraint is not being introduced by this migration. Since the column is nullable, NULL values are permitted and do not violate the foreign key constraint. The DROP CONSTRAINT IF EXISTS pattern suggests this FK was previously present and is being re-added.

Likely an incorrect or invalid review comment.

Comment thread sormas-backend/src/main/resources/sql/sormas_schema.sql Outdated
@sormas-vitagroup
Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
sormas-backend/src/main/resources/sql/sormas_schema.sql (1)

14724-14725: Foreign key constraint re-creation is idempotent but note naming inconsistency.

The DROP IF EXISTS followed by ADD CONSTRAINT pattern ensures idempotence. However, note that the constraint names differ between tables:

  • epidata uses fk_epidata_country_id (line 14725)
  • epidata_history uses fk_epi_country_id (line 14732)

If this naming convention is intentional per the existing schema, this is acceptable. Otherwise, consider standardizing constraint names for clarity. Both reference the same target table country(id), so the logic is sound.

Also applies to: 14731-14732

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d0c47e4 and f09355c.

📒 Files selected for processing (2)
  • sormas-backend/src/main/resources/sql/sormas_schema.sql (3 hunks)
  • sormas-backend/src/test/java/de/symeda/sormas/backend/caze/CaseClassificationLogicTest.java (14 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
  • GitHub Check: android app test (26)
  • GitHub Check: android app test (27)
  • GitHub Check: android app test (28)
  • GitHub Check: SORMAS CI
🔇 Additional comments (4)
sormas-backend/src/main/resources/sql/sormas_schema.sql (4)

14724-14727: Confirm type conversion safety and verify remaining data can be cast to float4.

The UPDATE statement sets invalid values to NULL before type conversion, which is a safe approach that prevents cast errors. The USING timeoffworkdays::float4 clause will then cleanly convert remaining valid numeric strings to float4, with NULLs remaining as NULL.

However, verify that:

  1. No non-NULL, non-matching values remain after the UPDATE (the regex and cleanup conditions should catch all invalid cases).
  2. The type conversion completes without errors post-deployment.
  3. The same cleanup logic is correctly applied to both symptoms (line 14726-14727) and symptoms_history (line 14733-14734) tables.

The regex check for spaces, the empty string check, and the decimal handling look appropriate for numeric validation.

Consider adding a post-migration verification test to confirm:

  • No NULL values were introduced unexpectedly for legitimate numeric strings
  • The type conversion succeeded for both tables

Also applies to: 14731-14734


14723-14723: Column additions are consistent and properly mirrored.

The animallocationtext column is correctly added to both exposures (line 14723) and exposures_history (line 14730) tables with matching VARCHAR(255) type. This aligns with the PR objective to add animalLocationText.

Also applies to: 14730-14730


14660-14698: New columns for Giardiasis/Cryptosporidiosis are consistently mirrored across main and history tables.

The additions to symptoms (lines 14660-14668) are properly mirrored to symptoms_history (lines 14690-14698). Same applies to exposures columns. The type conversions for weightlossamount are correctly applied to both table variants (lines 14668, 14698 using USING clause).


14722-14727: Regex pattern correctly handles valid numeric formats and aligns with domain semantics.

The regex ^\s*(?:\d+(?:\.\d*)?|\.\d+)\s*$ properly allows trailing decimals (e.g., "5.") compared to earlier iterations, addressing the prior review concern about unintended data loss. It intentionally excludes negative numbers, which aligns with domain semantics—time off work cannot be negative. The application code provides no validation annotations (@min, @positive) or business logic checks, yet the migration defensively filters negatives via regex, ensuring data consistency.

The data cleanup approach is sound: empty strings and non-matching values are safely set to NULL, preventing cast errors, while valid positive decimals convert cleanly to float4.

ALTER TABLE epidata ADD CONSTRAINT fk_epidata_country_id FOREIGN KEY (country_id) REFERENCES country (id);
UPDATE symptoms SET timeoffworkdays = NULL WHERE TRIM(timeoffworkdays) = '' OR timeoffworkdays !~ '^\s*(?:\d+(?:\.\d*)?|\.\d+)\s*$';
ALTER TABLE symptoms ALTER COLUMN timeoffworkdays TYPE float4 USING timeoffworkdays::float4;
ALTER TABLE healthconditions DROP COLUMN IF EXISTS immunodepression;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Immunodepression column removal aligns with PR objectives; verify no application code references this field.

Both healthconditions (line 14728) and healthconditions_history (line 14735) have the immunodepression column dropped, which matches the PR objective to remove immunodepression accessors.

Ensure that no Java entity classes, DTOs, or application logic still reference this field, as removing it without updating application code could cause runtime errors.

Also applies to: 14735-14735


🏁 Script executed:

#!/bin/bash
# Search for any remaining references to immunodepression in codebase

fd -e java -e ts -e tsx | xargs grep -n "immunodepression" -i

Length of output: 150


🏁 Script executed:

#!/bin/bash
# Search for references to the HealthConditions_immunodepression constant
rg -n "HealthConditions_immunodepression" 

# Also search for immunodepression in property/resource files
echo "=== Searching in property/resource files ==="
fd -e properties -e xml | xargs grep -n "immunodepression" -i

Length of output: 301


🏁 Script executed:

#!/bin/bash
# Search for any usage/references to the immunodepression caption key
rg -n "HealthConditions\.immunodepression" --type java --type properties --type xml

# Also check if the Captions constant is imported or used elsewhere
rg -n "HealthConditions_immunodepression" --type java

Length of output: 185


Remove orphaned i18n definitions for immunodepression field.

The PR successfully removed immunodepression database columns, and no Java entity/DTO field references remain in the codebase. However, two orphaned i18n artifacts were found:

  • sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java line 2008: String HealthConditions_immunodepression = "HealthConditions.immunodepression";
  • sormas-api/src/main/resources/captions.properties line 1654: HealthConditions.immunodepression=Immunodepression

These caption definitions are unused and should be removed to maintain consistency with the schema changes.

🤖 Prompt for AI Agents
In sormas-backend/src/main/resources/sql/sormas_schema.sql around line 14728 and
related files, remove the orphaned i18n entries for the deleted immunodepression
field: delete the constant String HealthConditions_immunodepression =
"HealthConditions.immunodepression"; from
sormas-api/src/main/java/de/symeda/sormas/api/i18n/Captions.java (around line
2008) and the property HealthConditions.immunodepression=Immunodepression from
sormas-api/src/main/resources/captions.properties (around line 1654); after
removal, run a project-wide search to confirm there are no remaining references
and run the build/tests to ensure no compile or i18n lookup issues.

@KarnaiahPesula KarnaiahPesula merged commit 14dbe7e into development Nov 4, 2025
6 of 12 checks passed
@KarnaiahPesula KarnaiahPesula deleted the 13610-cryptosporidium-enhance-case-data-form branch November 4, 2025 11:33
@sormas-vitagroup
Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cryptosporidiosis- Enhance case data form

3 participants